Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore destroy functionality #1690

Merged
merged 1 commit into from
Jun 30, 2020
Merged

Restore destroy functionality #1690

merged 1 commit into from
Jun 30, 2020

Conversation

nickcharlton
Copy link
Member

In #1618, we removed the explicit include of jquery_ujs as it should
no longer be necessary. Alas, this broke the ability to destroy items.

This wasn't caught by the tests because Capybara tries to be clever with
these links, sending the DELETE directly instead of clicking on the link. We
disable this here and switch the specs to use the JS driver so that
we're actually testing the functionality (it can't pass without).

Adding the include of jquery_ujs solves this for now, unblocking us
from having a release for lots of other features and allows us to
revisit this problem again (in a way that we'll catch it this time!).

Fixes #1643.

In #1618, we removed the explicit include of `jquery_ujs` as it should
no longer be necessary. Alas, this broke the ability to destroy items.

This wasn't caught by the tests because Capybara tries to be clever with
these links, sending the `DELETE` directly instead of clicking on the link. We
disable this here and switch the specs to use the JS driver so that
we're actually testing the functionality (it can't pass without).

Adding the include of `jquery_ujs` solves this for now, unblocking us
from having a release for lots of other features and allows us to
revisit this problem again (in a way that we'll catch it this time!).

Fixes #1643.
@pablobm pablobm mentioned this pull request Jun 28, 2020
@nickcharlton nickcharlton merged commit df3969f into master Jun 30, 2020
@nickcharlton nickcharlton deleted the nc-restore-destroy branch June 30, 2020 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Destroy link does not work
2 participants